Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cluster size validation rule #230

Merged
merged 7 commits into from
Nov 21, 2023

Conversation

grischperl
Copy link
Contributor

@grischperl grischperl commented Nov 21, 2023

Description

Changes proposed in this pull request:

Validation rule:

  • New cluster size can be set to 1
  • Once size has been greater than 1, size can no longer be set to 1

Related issue(s)

#210

@grischperl grischperl requested a review from a team as a code owner November 21, 2023 13:22
@kyma-bot kyma-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 21, 2023
@kyma-bot kyma-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 21, 2023
@grischperl grischperl linked an issue Nov 21, 2023 that may be closed by this pull request
@@ -125,6 +125,7 @@ type Cluster struct {
// +kubebuilder:default:=3
// +kubebuilder:validation:Minimum:=1
// +kubebuilder:validation:XValidation:rule="(self%2) != 0", message="size only accepts odd numbers"
// +kubebuilder:validation:XValidation:rule="!(oldSelf > 1 && self == 1)", message="size cannot be reduced to 1 once it was >1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message="size cannot be reduced to 1 once it was >1"
Wasn't this the other way around? Once it will be set to 1 it cannot be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the rule is, once it was set to greater than 1 it can never be 1 again. I can try to make the message clearer like cannot be set to 1 if size was greater than 1?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, looks I got that wrong. Nevermind then.

@@ -267,6 +267,36 @@ func Test_Validate_UpdateNATS(t *testing.T) {
},
wantErrMsg: "fileStorage is immutable once it was set",
},
{
name: `validation of cluster fails, if cluster.size>1 gets set to 1'`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is a little confusing. How about:

Suggested change
name: `validation of cluster fails, if cluster.size>1 gets set to 1'`,
name: `validation of cluster fails, if cluster.size was set to a value >1 and now gets changed to 1'`,

...or something similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@kyma-bot kyma-bot added the lgtm Looks good to me! label Nov 21, 2023
@kyma-bot kyma-bot merged commit 0940701 into kyma-project:main Nov 21, 2023
9 checks passed
@grischperl grischperl deleted the bug-cluster-size branch November 21, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Looks good to me! size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

decreasing nats cluster size to 1 destroys the cluster
3 participants